Skip to content

Feature implementation from commits 2d3126b..58c4866#2

Open
codeOwlAI wants to merge 15 commits intofeature-base-branch-2from
feature-head-branch-2
Open

Feature implementation from commits 2d3126b..58c4866#2
codeOwlAI wants to merge 15 commits intofeature-base-branch-2from
feature-head-branch-2

Conversation

@codeOwlAI
Copy link
Owner

@codeOwlAI codeOwlAI commented Jun 30, 2025

PR Summary

Network Protocol and Socket Handling Improvements

Overview

This PR enhances protocol detection reliability, improves socket option handling, and fixes several bugs across network-related components. Key improvements include protocol sniffing with timeouts, proper resource cleanup, and enhanced UDP address resolution.

Change Types

Type Description
Enhancement Improved protocol sniffing with timeout and better error handling
Bugfix Fixed resource leaks and corrected IP address formatting
Refactor Enhanced socket option handling and address resolution
Test Added comprehensive QUIC protocol sniffing test cases

Affected Modules

Module / File Change Description
dispatcher/default.go Added 200ms timeout for protocol sniffing with detailed error handling
dns/nameserver.go Fixed IP address formatting with proper type conversion
proxyman/outbound/handler.go Added resource cleanup in Handler's Close method
protocol/quic/sniff.go Modified error handling and added wipeBytes function (contains syntax error)
protocol/quic/sniff_test.go Added comprehensive test cases for QUIC sniffing
protocol/tls/sniff.go Enhanced TLS ClientHello parsing with QUIC validation
conf/transport_internet.go Added System field to CustomSockopt (contains typo: "Syetem")
internet/config.pb.go Added "system" field to CustomSockopt Protocol Buffer message
internet/sockopt_windows.go Improved IPv4 and multicast address detection
internet/system_dialer.go Added UDP address resolution and fixed destination address handling

Notes for Reviewers

  • The wipeBytes function in protocol/quic/sniff.go contains a syntax error in its for loop
  • There's a typo in conf/transport_internet.go (Syetem instead of System)
  • The destAddr variable in internet/system_dialer.go was removed but is still being used

xqzr and others added 15 commits April 7, 2025 11:50
Bumps [golang.org/x/net](https://github.com/golang/net) from 0.38.0 to 0.39.0.
- [Commits](golang/net@v0.38.0...v0.39.0)

---
updated-dependencies:
- dependency-name: golang.org/x/net
  dependency-version: 0.39.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [github.com/cloudflare/circl](https://github.com/cloudflare/circl) from 1.6.0 to 1.6.1.
- [Release notes](https://github.com/cloudflare/circl/releases)
- [Commits](cloudflare/circl@v1.6.0...v1.6.1)

---
updated-dependencies:
- dependency-name: github.com/cloudflare/circl
  dependency-version: 1.6.1
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* WireGuard: Fix tunnel not closed

* Dialer: Apply controllers in lc.Control
* Sockopt: Add custom sockopt on Windows & Darwin

* fix windows udp by the way

* use resolved addr

XTLS#4504 (comment)
Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.71.1 to 1.72.0.
- [Release notes](https://github.com/grpc/grpc-go/releases)
- [Commits](grpc/grpc-go@v1.71.1...v1.72.0)

---
updated-dependencies:
- dependency-name: google.golang.org/grpc
  dependency-version: 1.72.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [github.com/quic-go/quic-go](https://github.com/quic-go/quic-go) from 0.50.1 to 0.51.0.
- [Release notes](https://github.com/quic-go/quic-go/releases)
- [Changelog](https://github.com/quic-go/quic-go/blob/master/Changelog.md)
- [Commits](quic-go/quic-go@v0.50.1...v0.51.0)

---
updated-dependencies:
- dependency-name: github.com/quic-go/quic-go
  dependency-version: 0.51.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [github.com/refraction-networking/utls](https://github.com/refraction-networking/utls) from 1.6.7 to 1.7.1.
- [Release notes](https://github.com/refraction-networking/utls/releases)
- [Commits](refraction-networking/utls@v1.6.7...v1.7.1)

---
updated-dependencies:
- dependency-name: github.com/refraction-networking/utls
  dependency-version: 1.7.1
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
PSK extension, Post-Quantum Key Agreement, ML-KEM
…S#4642)

Co-authored-by: RPRX <63339210+RPRX@users.noreply.github.com>
Co-authored-by: Vigilans <vigilans@foxmail.com>
Co-authored-by: Shelikhoo <xiaokangwang@outlook.com>
Co-authored-by: dyhkwong <50692134+dyhkwong@users.noreply.github.com>
}

func wipeBytes(b []byte) {
for i := range len(b) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐛 Correctness Issue

Compilation Error: Invalid range syntax.

The code uses 'range len(b)' which is syntactically incorrect in Go and will fail to compile as range requires a slice, array, map, string or channel.

Current Code (Diff):

- 	for i := range len(b) {
+ 	for i := range b {
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
for i := range len(b) {
for i := range b {

// trailing dot. See
// https://tools.ietf.org/html/rfc6066#section-3.
if strings.HasSuffix(serverName, ".") {
if b == '.' {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐛 Correctness Issue

Incorrect trailing dot detection logic.

The new code only checks if the last byte is a period, which fails to properly implement the RFC6066 requirement for SNI values.

Current Code (Diff):

- 					if b == '.' {
+ 					if nameLen > 0 && d[nameLen-1] == '.' {
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
if b == '.' {
if nameLen > 0 && d[nameLen-1] == '.' {


for _, copt := range c.CustomSockopt {
customSockopt := &internet.CustomSockopt{
System: copt.Syetem,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐛 Correctness Issue

Typo in field name causes undefined behavior.

The code uses 'copt.Syetem' instead of 'copt.System' which will likely cause runtime errors or undefined behavior when configuring sockets.

Current Code (Diff):

-			System:  copt.Syetem,
+			System:  copt.System,
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
System: copt.Syetem,
System: copt.System,

isV4 := strings.Contains(address, ".")
// note: DO NOT trust the passed network variable, it can be udp6 even if the address is ipv4
// because operating system might(always) use ipv6 socket to process ipv4
host, _, err := net.SplitHostPort(address)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐛 Correctness Issue

Unchecked SplitHostPort error.

The error from net.SplitHostPort is not checked before using the host variable, which could cause panic if address format is invalid

Current Code (Diff):

+ 		host, _, err := net.SplitHostPort(address)
+ 		if err != nil {
+ 			return errors.New("failed to split host and port").Base(err)
+ 		}
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
host, _, err := net.SplitHostPort(address)
host, _, err := net.SplitHostPort(address)
if err != nil {
return errors.New("failed to split host and port").Base(err)
}

}
return &PacketConnWrapper{
Conn: packetConn,
Dest: destAddr,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐛 Correctness Issue

Undefined Variable Error: destAddr removed but still used.

The code removes the initialization of destAddr but still tries to use it in the PacketConnWrapper constructor, which will cause a compilation failure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants